Feat/oss product hardening - #67
Merged
wpak-ai merged 12 commits intoMay 13, 2026
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughCentralizes runtime config with a CLI, adds retry/timeout primitives and tests, introduces multi-format redacting logs, refactors PineconeClient for sparse indexes and per-call timeouts with retry wrappers, unifies query tools into a preset-driven tool, expands server exports, and updates CI, packaging, docs, and examples. ChangesConfiguration, CLI, and Utility Foundation
Logging and PineconeClient
Server, Tools, and Data Shapes
Sequence Diagram (high-level) sequenceDiagram
participant CLI
participant Config as ConfigResolver
participant Server
participant Pinecone
CLI->>Config: parseCli() -> resolveConfig()
Config->>Server: provide ServerConfig
Server->>Pinecone: PineconeClient.query/search (via runWithRetryTimeout)
Pinecone-->>Server: results
Server->>Server: normalize hits, rerank if needed
Server->>CLI: MCP response
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Cursor <cursoragent@cursor.com>
- Unify MCP query surface behind one tool with fast/detailed/full presets; update guided flow and docs. - Abort-aware withTimeout (reject-before-abort race, swallow stray fn rejections); add retry tests. - Pinecone client: shared hit mapping, keyword index namespace discriminated result, metadata filter and logger fixes; builtin URL generators register in setupServer. - README/CHANGELOG/.npmignore align with packaging and deployment model; vitest excludes glue layers from coverage gates. Co-authored-by: Cursor <cursoragent@cursor.com>
jonathanMLDev
force-pushed
the
feat/oss-product-hardening
branch
from
May 6, 2026 16:31
efcf64b to
f2d89f8
Compare
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
added 2 commits
May 8, 2026 02:26
11 tasks
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
AuraMindNest
approved these changes
May 13, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/suggestion-flow.ts (1)
5-6:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUpdate flow recommendation values to the new
querypreset contract.Line 5 and Line 58 still emit legacy tool names (
query_fast/query_detailed). With the unifiedquerytool, this can propagate invalid guidance to callers and tool-routing logic.Proposed contract update
type FlowState = { updatedAt: number; - recommended_tool: 'count' | 'query_fast' | 'query_detailed'; + recommended_tool: 'count' | 'fast' | 'detailed' | 'full'; suggested_fields: string[]; user_query: string; }; @@ flow: { updatedAt: Date.now(), - recommended_tool: 'query_fast', + recommended_tool: 'fast', suggested_fields: [], user_query: '', },Also applies to: 58-59
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/suggestion-flow.ts` around lines 5 - 6, The flow currently emits legacy tool names (query_fast / query_detailed) via the recommended_tool field, which must be unified to the new 'query' preset; update the type/union for recommended_tool and any code paths that set recommended_tool (look for references to recommended_tool and the function or block that builds the suggestion object near the emission around lines 58-59) to return 'query' instead of 'query_fast'/'query_detailed', and ensure callers/readers of suggested_fields or suggestion objects accept the new 'query' value so routing logic uses the unified tool name.
🧹 Nitpick comments (2)
src/server/url-generation.ts (1)
102-107: 💤 Low valueConsider setting the idempotent flag after successful registration.
The current implementation sets
builtinGeneratorsRegistered = truebefore callingurlGenerators.set(...). If an exception occurs during registration (unlikely withMap.set, but possible in future modifications), the flag would indicate completion while the registry remains incomplete.Best practice is to set the flag only after all registration operations succeed:
export function registerBuiltinUrlGenerators(): void { if (builtinGeneratorsRegistered) return; - builtinGeneratorsRegistered = true; urlGenerators.set('mailing', generatorMailing); urlGenerators.set('slack-Cpplang', generatorSlackCpplang); + builtinGeneratorsRegistered = true; }This ensures the guard reflects actual state even if the function is extended later.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/url-generation.ts` around lines 102 - 107, In registerBuiltinUrlGenerators, move the mutation of builtinGeneratorsRegistered so it is set to true only after all registrations succeed: perform urlGenerators.set('mailing', generatorMailing) and urlGenerators.set('slack-Cpplang', generatorSlackCpplang) first, then set builtinGeneratorsRegistered = true; this ensures the guard reflects successful completion and avoids marking the registry as initialized if an exception occurs during registration.CHANGELOG.md (1)
35-35: 💤 Low valueSimplify "API interface" to "API".
The phrase "identical API interface" is redundant since "API" stands for "Application Programming Interface." Consider:
-README "Comparison with Python Version" no longer claims an identical API interface; the new TypeScript-only tools +README "Comparison with Python Version" no longer claims an identical API; the new TypeScript-only tools🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 35, Update the changelog line that currently reads "identical API interface" to use the non-redundant term "API" instead; edit the sentence mentioning the new TypeScript-only tools (`guided_query`, `query_documents`, `keyword_search`, `namespace_router`, `suggest_query_params`, `count`, `generate_urls`) so it states they are listed explicitly and that the README no longer claims an "identical API" (remove the word "interface").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 12-17: The CI matrix currently only enumerates Node versions under
strategy.matrix.node-version while using a fixed runs-on value; update the
workflow to run an OS × Node matrix by adding strategy.matrix.os (e.g.,
["ubuntu-latest","windows-latest","macos-latest"]), change runs-on to use
matrix.os (runs-on: ${{ matrix.os }}), and keep or nest
strategy.matrix.node-version (node-version: [18.x,20.x,22.x]) so each OS is
tested across all Node versions; also scan the job for any OS-specific steps
that may need conditionals keyed on matrix.os and adjust them accordingly.
In `@README.md`:
- Around line 30-31: Update the top-level feature bullet describing the query
tool presets to list all supported values: change the text that currently reads
"`query` tool `preset=fast` / `detailed`" to include the unified `full` preset
(e.g., "`preset=fast | detailed | full`") so the README accurately reflects the
`query` tool's supported presets.
In `@src/cli.ts`:
- Around line 35-77: The CLI argument parsing currently treats any non-undefined
next token as a value, so flags like "--api-key --index-name foo" can corrupt
overrides; update the switch handling in the argument-parsing loop (the cases
for '--api-key', '--index-name', '--sparse-index-name', '--rerank-model',
'--top-k', '--log-level', '--log-format' and the later block around lines 79-94)
to only consume next when it is a legitimate value (e.g., next !== undefined &&
!next.startsWith('-')), and for '--top-k' additionally ensure parsePositiveInt
is only called when next is a valid non-flag token; apply this guarded
value-consumption pattern to all value-taking options to avoid accidentally
consuming subsequent flags.
In `@src/config.ts`:
- Around line 101-102: The config builder currently sets apiKey to an empty
string which yields an invalid ServerConfig later; update the
constructor/initializer that defines apiKey and indexName to validate required
values and throw an Error immediately when apiKey is missing (and similarly
validate indexName if it must be present). Specifically, in the block that
assigns apiKey and indexName (the variables named apiKey and indexName) and in
the repeated validation area around the ServerConfig creation (the later block
referenced around lines 125-137), replace silent defaults with explicit checks
that throw a clear error (e.g., "PINECONE_API_KEY is required") so the process
fails fast instead of returning an invalid ServerConfig.
In `@src/server/metadata-filter.ts`:
- Around line 36-41: The metadataFilterValueSchema is inconsistent with
isPrimitiveArray: isPrimitiveArray allows arrays of string|number|boolean but
metadataFilterValueSchema only permits string[] | number[]; update
metadataFilterValueSchema to accept boolean values as well (either add boolean[]
to the union or replace the array branch with an array of union(z.string(),
z.number(), z.boolean())) so the schema matches isPrimitiveArray's widened
primitive-array validation; ensure references to metadataFilterSchema continue
to use the updated metadataFilterValueSchema.
---
Outside diff comments:
In `@src/server/suggestion-flow.ts`:
- Around line 5-6: The flow currently emits legacy tool names (query_fast /
query_detailed) via the recommended_tool field, which must be unified to the new
'query' preset; update the type/union for recommended_tool and any code paths
that set recommended_tool (look for references to recommended_tool and the
function or block that builds the suggestion object near the emission around
lines 58-59) to return 'query' instead of 'query_fast'/'query_detailed', and
ensure callers/readers of suggested_fields or suggestion objects accept the new
'query' value so routing logic uses the unified tool name.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 35: Update the changelog line that currently reads "identical API
interface" to use the non-redundant term "API" instead; edit the sentence
mentioning the new TypeScript-only tools (`guided_query`, `query_documents`,
`keyword_search`, `namespace_router`, `suggest_query_params`, `count`,
`generate_urls`) so it states they are listed explicitly and that the README no
longer claims an "identical API" (remove the word "interface").
In `@src/server/url-generation.ts`:
- Around line 102-107: In registerBuiltinUrlGenerators, move the mutation of
builtinGeneratorsRegistered so it is set to true only after all registrations
succeed: perform urlGenerators.set('mailing', generatorMailing) and
urlGenerators.set('slack-Cpplang', generatorSlackCpplang) first, then set
builtinGeneratorsRegistered = true; this ensures the guard reflects successful
completion and avoids marking the registry as initialized if an exception occurs
during registration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30030c30-5fc1-4e21-9a59-26e5d4d28308
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
.env.example.gitattributes.github/dependabot.yml.github/workflows/ci.yml.gitignore.npmignore.prettierrcCHANGELOG.mdREADME.mdexamples/README.mdexamples/custom-url-generator.tspackage.jsonscripts/test-search.tssrc/cli.tssrc/config.tssrc/constants.tssrc/index.tssrc/logger.tssrc/pinecone-client.tssrc/server.test.tssrc/server.tssrc/server/config-context.tssrc/server/format-query-result.tssrc/server/metadata-filter.tssrc/server/namespaces-cache.tssrc/server/query-suggestion.tssrc/server/retry.test.tssrc/server/retry.tssrc/server/suggestion-flow.tssrc/server/tools/count-tool.tssrc/server/tools/guided-query-tool.tssrc/server/tools/keyword-search-tool.tssrc/server/tools/list-namespaces-tool.tssrc/server/tools/query-documents-tool.tssrc/server/tools/query-tool.tssrc/server/tools/suggest-query-params-tool.tssrc/server/url-generation.test.tssrc/server/url-generation.tssrc/types.tstsconfig.jsonvitest.config.ts
wpak-ai
approved these changes
May 13, 2026
This was referenced Jun 2, 2026
2 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OSS product hardening (configuration, resilience, unified query, CI)
Pull request: #67
Branch:
feat/oss-product-hardeningTracking issue (local): #71
Summary
This pull request hardens the TypeScript read-only Pinecone MCP server for real OSS and production use: centralized configuration (CLI + environment + defaults), safer multi-format logging with API key redaction, shared retry and abort-aware timeouts around Pinecone SDK calls, a single preset-driven
queryMCP tool (replacing separate fast/detailed variants), clearer guided-query orchestration, packaging and documentation updates, and a stronger CI matrix (coverage, Codecov, SBOM, Dependabot grouping).Motivation
preset:fast|detailed|full) instead of multiple overlapping tools.User-visible changes
MCP
querytool withpresetmapping to prior fast/detailed/full behaviors;guided_querypreferred_toolsupportsauto,count,fast,detailedwith routing aligned to suggestions.list_namespacesmay includeexpires_at_isofor cache transparency.document_id(legacypaper_numbertreated as deprecated where applicable).CLI / process
--help,--version, and CLI overrides for key settings where implemented.process.exitpatterns on recoverable paths (details in commits).Configuration (environment)
New or expanded variables (names may match
PINECONE_*or project-prefixed vars as in.env.example):See
.env.exampleand README for the canonical list and semantics.Library / types
PineconeClientand types updated for sparse index name, timeouts, normalized hit mapping, and discriminated results for keyword-index namespace listing where applicable.$in/$ninwith string, number, or boolean arrays.Docs and packaging
.npmignoreadjusted so published packages exclude bulkydocs/while shipping runtime artifacts..gitattributes/ LF hygiene for Prettier and consistent checkouts.CI
Breaking changes
query_fast/query_detailedstyle tools are consolidated behind onequerytool withpreset. Update client configs and prompts that referenced old tool names.PineconeClientand related types should follow CHANGELOG notes for constructor options, return shapes, and exports.How to test
Manual smoke (optional): run the server with
.envfrom.env.example, invokelist_namespaces,suggest_query_params, unifiedquerywith each preset, andguided_querywithpreferred_toolautoand explicit modes.Checklist
.env.example)Related
close #71
Summary by CodeRabbit
New Features
--helpand--versionflags.document_idfield in query results.Changes
Chores